Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40617 +/- ##
=============================================
+ Coverage 56.16% 79.81% +23.64%
=============================================
Files 1124 494 -630
Lines 178336 83581 -94755
Branches 1489 0 -1489
=============================================
- Hits 100170 66710 -33460
+ Misses 75645 16871 -58774
+ Partials 2521 0 -2521
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/gemini review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This PR updates the Beam YAML testing documentation to explicitly note that both --tests and --test CLI flags are supported, fixes minor typos in the documentation, and expands unit test coverage in main_test.py to verify argument parsing and test execution for both flag variants.
🔍 General Feedback
- The unit test additions in
sdks/python/apache_beam/yaml/main_test.pycleanly validate both--testsand--testflags via_parse_argumentsandrun_tests. - Documentation updates and typo fixes in
website/www/site/content/en/documentation/sdks/yaml-testing.mdare clear and accurate.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @chamikaramj for label yaml. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
|
||
| def test_inline_test_specs(self): | ||
| main.run_tests(['--yaml_pipeline', TEST_PIPELINE, '--test'], exit=False) | ||
| for flag in ['--tests', '--test']: |
There was a problem hiding this comment.
Why do we have 2 different ways of doing this? Could you add some context?
There was a problem hiding this comment.
- Originally
--testwas added. - Then docs were added stating
--testswas to be used. - Another person added
--testsand kept the backward compatible--test. - This PR completes the cycle with improved testing for both flags and update documentation.
There was a problem hiding this comment.
Ok - I think the alias is fine (it probably would've been better to just update the docs, but that ship has sailed). But instead of alluding to both in our docs, lets just pick one and make sure all of our docs use that. I don't care which one we choose, but I think mentioning both (or switching back and forth) will just confuse users and lead them to wonder which one they should choose.
Keeping the main_test.py change is fine
There was a problem hiding this comment.
Ok removed the --test change. Thanks
|
waiting on author |
Fixes #37980 and some minor typos.